Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fully qualify type references; remove usings #60

Merged

Conversation

simonmckenzie
Copy link
Contributor

This updates the interface generation to always fully qualify type references, and no longer generate "using" statements in generated interfaces.

This makes the interface generation resilient to namespace collisions and other namespace edge-cases (like resolving global usings in parent assemblies). It also means the generator is now able to correctly resolve types with duplicate names

Addresses #55

public partial interface IDemoClass
{
/// <inheritdoc />
string CMethod<T, T1, T2, T3, T4>(string x, string y) where T : class where T1 : struct where T3 : DemoClass where T4 : IDemoClass;
string CMethod<T, T1, T2, T3, T4>(string x, string y) where T : class where T1 : struct where T3 : global::AutomaticInterfaceExample.DemoClass where T4 : IDemoClass;
Copy link
Contributor Author

@simonmckenzie simonmckenzie Aug 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the one place where types aren't fully qualified (where T4: IDemoClass).

IDemoClass doesn't exist at the time of generation, so it can't be resolved. This is OK though, since the reference is within the scope of IDemoClass, so no collision is possible

@@ -18,6 +18,15 @@ public static class Builder
| SymbolDisplayParameterOptions.IncludeParamsRefOut
);

private static readonly SymbolDisplayFormat TypeDisplayFormat =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New display format for types. Based on SymbolDisplayFormat.FullyQualifiedFormat, but with support for nullables, and without unnecessary non-type parameters

cb.AppendLine($"namespace {nameSpaceName}");
cb.AppendLine("{");

cb.Indent();

cb.AppendAndNormalizeMultipleLines(classDocumentation);

cb.AppendLine($"[GeneratedCode(\"AutomaticInterface\", \"\")]");
cb.AppendLine(
"[global::System.CodeDom.Compiler.GeneratedCode(\"AutomaticInterface\", \"\")]"
Copy link
Contributor Author

@simonmckenzie simonmckenzie Aug 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a choice - it seemed simpler and more concise to fully qualify this too, since otherwise there's only be one using in the interface file, using System.CodeDom.Compiler;

If you don't like this choice, just say and I'll change it back :)

@simonmckenzie simonmckenzie force-pushed the fix/fully-qualify-types branch 2 times, most recently from a4af269 to f2df1bc Compare August 21, 2024 01:35
@@ -257,61 +245,12 @@ public partial interface IDemoClass
GenerateCode(code).Should().Be(expected);
}

[Fact]
public void AddsUsingsToInterface()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obsolete test

}

[Fact]
public void WorksWithFileScopedNamespace()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved into GeneratorTests.TypeResolution

This updates the interface generation to always fully qualify type references, and no longer generate "using" statements in generated interfaces.

This makes the interface generation resilient to namespace collisions and other namespace edge-cases (like resolving global usings in parent assemblies). It also means the generator is now able to correctly resolve types with duplicate names
@simonmckenzie simonmckenzie force-pushed the fix/fully-qualify-types branch from c90392a to 296d31c Compare August 24, 2024 02:10
Without `global::` types in interfaces, the generated interface will resolve to the locally-defined type instead of the global one
The line above already asserts that there should be no errors without using `MissingUsingsAreOk`, so the extra check does nothing
Comment on lines -37 to -42
sourceDiagnostics
.Where(d => d.Severity == DiagnosticSeverity.Error)
.Where(x => x.Id != "CS0246")
.Where(x => !MissingUsingsAreOk(x))
.Should()
.BeEmpty();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check was already covered in the broader check on lines 31-35:

sourceDiagnostics
.Where(d => d.Severity == DiagnosticSeverity.Error)
.Where(x => x.Id != "CS0246") // missing references are ok
.Should()
.BeEmpty();

@ChristianSauer ChristianSauer merged commit e151b05 into codecentric:master Sep 1, 2024
3 checks passed
@ChristianSauer
Copy link
Collaborator

Looks very good, thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants